Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added a different way of vector intialization. #1787

Closed
wants to merge 1 commit into from

Conversation

mani-chand
Copy link
Contributor

I am taking example datatype as String you can intialize with any datatype that is supported by rust.
Specifying Vec::<String>::new() explicitly indicates that variable is intended to be a vector containing String elements. This can be helpful for documenting the code and ensuring that variable is used correctly throughout the codebase.

Specifying ```Vec::<String>::new()``` explicitly indicates that variable is intended to be a vector containing String elements. This can be helpful for documenting the code and ensuring that variable is used correctly throughout the codebase.
Copy link
Collaborator

@djmitche djmitche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is adding anything to the course. In general, changes to the text should come with some rationale: either missed content, out-of-order content, or observed student confusion.

Copy link
Collaborator

@djmitche djmitche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, forgot to click "Save" on the comment :)

@@ -20,6 +20,9 @@ fn main() {
// Canonical macro to initialize a vector with elements.
let mut v3 = vec![0, 0, 1, 2, 3, 4];

// specifying the type when initializing a vector.
let mut v4 = Vec::<String>::new();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A more idiomatic way to do this would be

    let mut v4: Vec<String> = Vec::new();

which was covered in the type-inference slide on day 1.

The turbofish operator (::<String>) is introduced in the speaker notes in the impl trait slide in the "Generics" segment and further refined in the sections on FromIterator. I think that's enough coverage for the moment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A more idiomatic way to do this would be

    let mut v4: Vec<String> = Vec::new();

which was covered in the type-inference slide on day 1.

The turbofish operator (::<String>) is introduced in the speaker notes in the impl trait slide in the "Generics" segment and further refined in the sections on FromIterator. I think that's enough coverage for the moment.

Oh! That's my bad i haven't noticed it. Then lets just close the PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you say.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good!

@djmitche
Copy link
Collaborator

djmitche commented Feb 6, 2024

@mani-chand what do you think about doing some work on #1464? A PR fixing the sizes of the slides in a single segment would be a good place to start. This is something really valuable to students, as they can see the course content on the screen without scrolling.

I noticed yesterday while making some other changes that the "Control Flow Basics" segment has some very long slides.

If you're interested, make a comment in that issue and I'll assign it.

@mani-chand
Copy link
Contributor Author

mani-chand commented Feb 6, 2024

@mani-chand what do you think about doing some work on #1464? A PR fixing the sizes of the slides in a single segment would be a good place to start. This is something really valuable to students, as they can see the course content on the screen without scrolling.

I noticed yesterday while making some other changes that the "Control Flow Basics" segment has some very long slides.

If you're interested, make a comment in that issue and I'll assign it.

Sure @djmitche , I am interested in working on it.

@djmitche djmitche closed this Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants